fix handling of URIs in the default namespace#2365
Conversation
…ceManager.curie() - including tests for both methods, including expected failure-cases for expand_curie(). - this should fix RDFLib#2348 - also addresses the converse case from RDFLib#2348, mapping a URI in the default namespace to a CURIE, using a new method to avoid a breaking change to qname().
* Changed the tests to use an in-module fixture instead of the `test_ontology.owl` graph and removed the `test_ontology.owl` file. The problem with arbitrary test data is that it is hard to evolve. If possible test data should be kept as terse as possible. In this case I just created a namespace manager with three namespaces added, of which only two will be effective because of <RDFLib#2077>. * Added a warning to `NamespaceManager.curie` to warn users that it is not a pure function and explain the specific side effects.
|
@jclerman please see if you are okay with 598f967
If so then I'm happy to merge it, also happy to make further adjustments, just let me know. |
|
The point about Does that default sound OK, or is it too confusing to have the default on |
A parameter like that would be nice, but I would not say essential. If the namespace manager was a blank slate, I would prefer to default to So, it is up to you if you want to add a generate parameter to this PR, happy if you change this PR to add it. I would though say it should default to |
|
All your comments make sense. I think the generate option would be very useful, so I'm adding it in a new commit here, with the default=True for the reason you mentioned (and I agree with everything you said). I updated the test function to test out the 3 possible settings of the parameter - not passed, True, and False. |
I forgot to remove this when I removed the test data.
I added some qualification to the warning and placed booleans in single backticks.
aucampia
left a comment
There was a problem hiding this comment.
LGTM, thanks for fixing this bug and the new functionality.
Fixes: #2348
Adds failing test-case for: #2077 (marked xfail for now)
Summary of changes
Addressing two issues with NamespaceManager:
NamespaceManager.expand_curie()did not support expansion of CURIEs with a leading colon, even though those are valid and should resolve using the default namespace for a graph.expand_curie()from a URI in the default namespace. The new support is added with a new method,curie()on NamespaceManager, so as not to change (break) existing behavior ofNamespaceManager.qname().Tests are included for both methods, including expected failure-cases for expand_curie().
qname().Checklist
the same change.
./examplesfor new features.CHANGELOG.md).so maintainers can fix minor issues and keep your PR up to date.